Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always capture tokens for macro_rules! arguments #73293

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

Aaron1011
Copy link
Member

@Aaron1011 Aaron1011 commented Jun 12, 2020

When we invoke a proc-macro, the TokenStream we pass to it may contain 'interpolated' AST fragments, represented by rustc_ast::token::Nonterminal. In order to correctly, pass a Nonterminal to a proc-macro, we need to have 'captured' its TokenStream at the time the AST was parsed.

Currently, we perform this capturing when attributes are present on items and expressions, since we will end up using a Nonterminal to pass the item/expr to any proc-macro attributes it is annotated with. However, Nonterminals are also introduced by the expansion of metavariables in macro_rules! macros. Since these metavariables may be passed to proc-macros, we need to have tokens available to avoid the need to pretty-print and reparse (see #43081).

This PR unconditionally performs token capturing for AST items and expressions that are passed to a macro_rules! invocation. We cannot know in advance if captured item/expr will be passed to proc-macro, so this is needed to ensure that tokens will always be available when they are needed.

This ensures that proc-macros will receive tokens with proper Spans (both location and hygiene) in more cases. Like all work on #43081, this will cause regressions in proc-macros that were relying on receiving tokens with dummy spans.

In this case, Crater revealed only one regression: the Pear crate (a helper for rocket), which was previously fixed as part of #73084.

This regression manifests itself as the following error:

[INFO] [stdout] error: proc macro panicked
[INFO] [stdout]    --> /opt/rustwide/cargo-home/registry/src/github.com-1ecc6299db9ec823/rocket_http-0.4.5/src/parse/uri/parser.rs:119:34
[INFO] [stdout]     |
[INFO] [stdout] 119 |             let path_and_query = pear_try!(path_and_query(is_pchar));
[INFO] [stdout]     |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[INFO] [stdout]     |
[INFO] [stdout]     = help: message: called `Option::unwrap()` on a `None` value
[INFO] [stdout]     = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

It can be fixed by running cargo update -p pear, which updates your Cargo.lock to use the latest version of Pear (which includes a bugfix for the regression).

Split out from #73084

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 12, 2020
@Aaron1011
Copy link
Member Author

r? @petrochenkov

@Aaron1011 Aaron1011 force-pushed the feature/macro-rules-arg-capture branch from 4d4ca54 to 5987141 Compare June 12, 2020 22:16
@rust-highfive

This comment has been minimized.

@Aaron1011 Aaron1011 force-pushed the feature/macro-rules-arg-capture branch from 5987141 to 5f5636f Compare June 13, 2020 00:34
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 14, 2020
@Aaron1011 Aaron1011 force-pushed the feature/macro-rules-arg-capture branch from 5f5636f to 74599cd Compare June 15, 2020 15:51
@Aaron1011
Copy link
Member Author

@petrochenkov: I've addressed your comments.

@Aaron1011 Aaron1011 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2020
@petrochenkov
Copy link
Contributor

LGTM.

I don't expect breakage, but there may be surprises.
Let's run this through crater.
@bors try

@bors
Copy link
Contributor

bors commented Jun 16, 2020

⌛ Trying commit 74599cd with merge 8bd52ddba12ca76b95d401142c1bd190bda1fd2a...

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2020
@bors
Copy link
Contributor

bors commented Jun 16, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jun 16, 2020
@Aaron1011
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Jun 16, 2020

⌛ Trying commit 74599cd with merge 6213129f69f3dab2041bcf4ba3c00595328028c0...

@Aaron1011 Aaron1011 removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 16, 2020
@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 24, 2020
@bors bors merged commit 3c90ae8 into rust-lang:master Jun 24, 2020
yammmt added a commit to yammmt/last-date that referenced this pull request Jun 25, 2020
Updating `pear` fixes broken build by nightly Rust update.
For detail, see rust-lang/rust#73293
yammmt added a commit to yammmt/shokuzai that referenced this pull request Jun 25, 2020
Updating `pear` fixes broken build by nightly Rust update.
For detail, see rust-lang/rust#73293
@nnethercote
Copy link
Contributor

This was a perf loss on the artificial deep-vector benchmark, which probably doesn't matter much.

@francois-blanchard
Copy link

I had the same problem during the rocket compilation.
cargo update -p pear and cargo update -p pear_codegen are fixed my issue.
Thx @Aaron1011.

Rust version

$ rustc --version
rustc 1.46.0-nightly (23744c84d 2020-07-14)

Cargo.toml

[dependencies]
rocket = "0.4.5"
...
pear = "0.1.4"
pear_codegen = "0.1.4"

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 20, 2020
This extends PR rust-lang#73293 to handle patterns (Pat). Unlike expressions,
patterns do not support custom attributes, so we only need to capture
tokens during macro_rules! argument parsing.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2020
…trochenkov

Capture tokens for Pat used in macro_rules! argument

This extends PR rust-lang#73293 to handle patterns (Pat). Unlike expressions,
patterns do not support custom attributes, so we only need to capture
tokens during macro_rules! argument parsing.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Aug 28, 2020
This extends PR rust-lang#73293 to handle patterns (Pat). Unlike expressions,
patterns do not support custom attributes, so we only need to capture
tokens during macro_rules! argument parsing.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 30, 2020
according to various people on tech-pkg@, there are no problems with
the Firefox build

Version 1.46.0 (2020-08-27)
==========================

Language
--------
- [`if`, `match`, and `loop` expressions can now be used in const functions.][72437]
- [Additionally you are now also able to coerce and cast to slices (`&[T]`) in
  const functions.][73862]
- [The `#[track_caller]` attribute can now be added to functions to use the
  function's caller's location information for panic messages.][72445]
- [Recursively indexing into tuples no longer needs parentheses.][71322] E.g.
  `x.0.0` over `(x.0).0`.
- [`mem::transmute` can now be used in static and constants.][72920] **Note**
  You currently can't use `mem::transmute` in constant functions.

Compiler
--------
- [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516]
- [Enabled static "Position Independent Executables" by default
  for `x86_64-unknown-linux-musl`.][70740]

Libraries
---------
- [`mem::forget` is now a `const fn`.][73887]
- [`String` now implements `From<char>`.][73466]
- [The `leading_ones`, and `trailing_ones` methods have been stabilised for all
  integer types.][73032]
- [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583]
- [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their
  zero-able equivalent (e.g. `TryFrom<u8>`).][72717]
- [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660]
- [`(String, u16)` now implements `ToSocketAddrs`.][73007]
- [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584]

Stabilized APIs
---------------
- [`Option::zip`]
- [`vec::Drain::as_slice`]

Cargo
-----
Added a number of new environment variables that are now available when
compiling your crate.

- [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of
  the specific binary being compiled and the name of the crate.
- [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the package.
- [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file.

Compatibility Notes
-------------------
- [The target configuration option `abi_blacklist` has been renamed
  to `unsupported_abis`.][74150] The old name will still continue to work.
- [Rustc will now warn if you cast a C-like enum that implements `Drop`.][72331]
  This was previously accepted but will become a hard error in a future release.
- [Rustc will fail to compile if you have a struct with
  `#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently only
  allowed on `enum`s.
- [Tokens passed to `macro_rules!` are now always captured.][73293] This helps
  ensure that spans have the correct information, and may cause breakage if you
  were relying on receiving spans with dummy information.
- [The InnoSetup installer for Windows is no longer available.][72569] This was
  a legacy installer that was replaced by a MSI installer a few years ago but
  was still being built.
- [`{f32, f64}::asinh` now returns the correct values for negative numbers.][72486]
- [Rustc will no longer accept overlapping trait implementations that only
  differ in how the lifetime was bound.][72493]
- [Rustc now correctly relates the lifetime of an existential associated
  type.][71896] This fixes some edge cases where `rustc` would erroneously allow
  you to pass a shorter lifetime than expected.
- [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420]
  The library will need to be installed for `rustc` to work, even though we
  expect it to be already available on most systems.
- [Tests annotated with `#[should_panic]` are broken on ARMv7 while running
  under QEMU.][74820]
- [Pretty printing of some tokens in procedural macros changed.][75453] The
  exact output returned by rustc's pretty printing is an unstable
  implementation detail: we recommend any macro relying on it to switch to a
  more robust parsing system.

[75453]: rust-lang/rust#75453
[74820]: rust-lang/rust#74820
[74420]: rust-lang/rust#74420
[74109]: rust-lang/rust#74109
[74150]: rust-lang/rust#74150
[73862]: rust-lang/rust#73862
[73887]: rust-lang/rust#73887
[73466]: rust-lang/rust#73466
[73516]: rust-lang/rust#73516
[73293]: rust-lang/rust#73293
[73007]: rust-lang/rust#73007
[73032]: rust-lang/rust#73032
[72920]: rust-lang/rust#72920
[72569]: rust-lang/rust#72569
[72583]: rust-lang/rust#72583
[72584]: rust-lang/rust#72584
[72717]: rust-lang/rust#72717
[72437]: rust-lang/rust#72437
[72445]: rust-lang/rust#72445
[72486]: rust-lang/rust#72486
[72493]: rust-lang/rust#72493
[72331]: rust-lang/rust#72331
[71896]: rust-lang/rust#71896
[71660]: rust-lang/rust#71660
[71322]: rust-lang/rust#71322
[70740]: rust-lang/rust#70740
[cargo/8270]: rust-lang/cargo#8270
[cargo/8325]: rust-lang/cargo#8325
[cargo/8387]: rust-lang/cargo#8387
[`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip
[`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
j6k4m8 added a commit to aplbrain/bossphorus that referenced this pull request Sep 2, 2020
j6k4m8 added a commit to aplbrain/bossphorus that referenced this pull request Sep 4, 2020
* Fix extra cuboid downloads

* Rust fmt linting

* Cargo-update to include rust-lang/rust#73293
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 30, 2020
Pkgsrc changes:
 * Portability patches for Illumos have been intregrated upstream,
   so are no longer needed in pkgsrc.
 * Adjust one other patch, and update vendor/libc cargo checksum.

Upstream changes:

Version 1.46.0 (2020-08-27)
==========================

Language
--------
- [`if`, `match`, and `loop` expressions can now be used in const functions.]
  [72437]
- [Additionally you are now also able to coerce and cast to slices (`&[T]`) in
  const functions.][73862]
- [The `#[track_caller]` attribute can now be added to functions to use the
  function's caller's location information for panic messages.][72445]
- [Recursively indexing into tuples no longer needs parentheses.][71322] E.g.
  `x.0.0` over `(x.0).0`.
- [`mem::transmute` can now be used in static and constants.][72920] **Note**
  You currently can't use `mem::transmute` in constant functions.

Compiler
--------
- [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516]
- [Enabled static "Position Independent Executables" by default
  for `x86_64-unknown-linux-musl`.][70740]

Libraries
---------
- [`mem::forget` is now a `const fn`.][73887]
- [`String` now implements `From<char>`.][73466]
- [The `leading_ones`, and `trailing_ones` methods have been stabilised for all
  integer types.][73032]
- [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583]
- [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their
  zero-able equivalent (e.g. `TryFrom<u8>`).][72717]
- [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660]
- [`(String, u16)` now implements `ToSocketAddrs`.][73007]
- [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584]

Stabilized APIs
---------------
- [`Option::zip`]
- [`vec::Drain::as_slice`]

Cargo
-----
Added a number of new environment variables that are now available when
compiling your crate.

- [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of
  the specific binary being compiled and the name of the crate.
- [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the
  package.
- [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file.

Compatibility Notes
-------------------
- [The target configuration option `abi_blacklist` has been renamed
  to `unsupported_abis`.][74150] The old name will still continue to work.
- [Rustc will now warn if you have a C-like enum that implements `Drop`.][72331]
  This was previously accepted but will become a hard error in a future release.
- [Rustc will fail to compile if you have a struct with
  `#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently
  only allowed on `enum`s.
- [Tokens passed to `macro_rules!` are now always captured.][73293] This helps
  ensure that spans have the correct information, and may cause breakage if you
  were relying on receiving spans with dummy information.
- [The InnoSetup installer for Windows is no longer available.][72569] This was
  a legacy installer that was replaced by a MSI installer a few years ago but
  was still being built.
- [`{f32, f64}::asinh` now returns the correct values for negative numbers.]
  [72486]
- [Rustc will no longer accept overlapping trait implementations that only
  differ in how the lifetime was bound.][72493]
- [Rustc now correctly relates the lifetime of an existential associated
  type.][71896] This fixes some edge cases where `rustc` would erroneously
  allow you to pass a shorter lifetime than expected.
- [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420]
  The library will need to be installed for `rustc` to work, even though we
  expect it to be already available on most systems.
- [Tests annotated with `#[should_panic]` are broken on ARMv7 while running
  under QEMU.][74820]
- [Pretty printing of some tokens in procedural macros changed.][75453] The
  exact output returned by rustc's pretty printing is an unstable
  implementation detail: we recommend any macro relying on it to switch to a
  more robust parsing system.

[75453]: rust-lang/rust#75453
[74820]: rust-lang/rust#74820
[74420]: rust-lang/rust#74420
[74109]: rust-lang/rust#74109
[74150]: rust-lang/rust#74150
[73862]: rust-lang/rust#73862
[73887]: rust-lang/rust#73887
[73466]: rust-lang/rust#73466
[73516]: rust-lang/rust#73516
[73293]: rust-lang/rust#73293
[73007]: rust-lang/rust#73007
[73032]: rust-lang/rust#73032
[72920]: rust-lang/rust#72920
[72569]: rust-lang/rust#72569
[72583]: rust-lang/rust#72583
[72584]: rust-lang/rust#72584
[72717]: rust-lang/rust#72717
[72437]: rust-lang/rust#72437
[72445]: rust-lang/rust#72445
[72486]: rust-lang/rust#72486
[72493]: rust-lang/rust#72493
[72331]: rust-lang/rust#72331
[71896]: rust-lang/rust#71896
[71660]: rust-lang/rust#71660
[71322]: rust-lang/rust#71322
[70740]: rust-lang/rust#70740
[cargo/8270]: rust-lang/cargo#8270
[cargo/8325]: rust-lang/cargo#8325
[cargo/8387]: rust-lang/cargo#8387
[`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip
[`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.